Skip to content

fix: emit the wire request as raw UTF-8 so the challenge id reproduces - #185

Open
EfeDurmaz16 wants to merge 2 commits into
tempoxyz:mainfrom
EfeDurmaz16:fix/canonical-json-non-ascii
Open

fix: emit the wire request as raw UTF-8 so the challenge id reproduces#185
EfeDurmaz16 wants to merge 2 commits into
tempoxyz:mainfrom
EfeDurmaz16:fix/canonical-json-non-ascii

Conversation

@EfeDurmaz16

@EfeDurmaz16 EfeDurmaz16 commented Jul 28, 2026

Copy link
Copy Markdown

The bug

generate_challenge_id() serializes the request with ensure_ascii=False, but the code that writes
the request parameter into the header does not. For any non-ASCII character the HMAC signs one
string while the header carries another, so a peer cannot recompute the id and rejects the challenge
with invalid-challenge.

Section 5.1.1 of draft-httpauth-payment-00 binds the id to the request as it appears on the wire,
so the two have to be the same bytes.

Sites that had drifted to the ensure_ascii=True default:

  • _parsing.py:43 (_b64_encode), reached from format_www_authenticate at :169
  • extensions/mcp/types.py:92,190,195

Before

sequenceDiagram
    autonumber
    participant S as pympp server
    participant P as Peer verifier
    Note over S: request contains a non-ASCII character
    S->>S: generate_challenge_id(), ensure_ascii=False
    Note right of S: id = HMAC(raw UTF-8 bytes)
    S->>S: format_www_authenticate(), ensure_ascii=True
    Note right of S: header carries escaped bytes
    S-->>P: 402 with that id, but escaped request
    P->>P: recompute HMAC from the header value
    Note right of P: HMAC(escaped bytes), does not match
    P-->>S: rejected, invalid-challenge
Loading

After

sequenceDiagram
    autonumber
    participant S as pympp server
    participant P as Peer verifier
    Note over S: request contains a non-ASCII character
    S->>S: generate_challenge_id() via canonical_json()
    S->>S: format_www_authenticate() via canonical_json()
    Note right of S: one definition, same bytes
    S-->>P: 402 with that id, raw UTF-8 request
    P->>P: recompute HMAC from the header value
    Note right of P: same bytes, matches
    P-->>S: accepted
Loading

Why the tests missed it

verify() decodes the echoed request back to a dict and re-serializes it before recomputing the id,
so pympp always agrees with itself. The mismatch only appears against another implementation.

test_unicode_in_description covers generate_challenge_id(), which was already correct. Nothing
covered the header.

The fix

The same serialization rule appeared at nine call sites and four of them had drifted. This routes
all nine through one canonical_json() helper.

Read _parsing.py first, it holds the fix. Then __init__.py, extensions/mcp/types.py and
_body_digest.py, which just call the helper, then the tests.

Commit 1 is the behavior change, commit 2 is the tests. Both pass on their own.

Testing

make lint && make format-check && make test

780 passed, 41 skipped (778 on main, plus the 2 new tests).

To watch the tests fail without the fix, cherry-pick the test commit onto main and run
uv run pytest tests/test_challenge_id.py -k WireForm.

One thing worth a second opinion

The smallest possible fix is four ensure_ascii=False arguments. I used a shared helper instead,
which touches __init__.py and _body_digest.py even though they were already correct. My reasoning
is that nine copies of one rule, four of them drifted, is the actual problem. If you would rather
keep the diff minimal, I will cut it back.

Only non-ASCII requests change. ASCII output is byte-identical, so existing challenge ids are
unaffected, and the ids that do change are ones no other implementation could verify anyway.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

generate_challenge_id() canonicalizes the request with ensure_ascii=False,
but _b64_encode(), which format_www_authenticate() uses to build the wire
`request` auth-param, relied on json.dumps' ensure_ascii default of True.
For any non-ASCII byte the HMAC bound one string while the header carried
another, so the challenge id could not be reproduced from the value on the
wire.

Section 5.1.1 of draft-httpauth-payment-00 binds the id to the
base64url-encoded request as it appears on the wire, so the HMAC input and
the emitted parameter have to be the same bytes.

This SDK still verifies its own challenges because verify() decodes the
echoed value back to a dict and re-canonicalizes it, which is why the
mismatch was invisible locally and only surfaced against other SDKs.

The same rule was duplicated across nine call sites and four of them had
drifted. Route them all through a single canonical_json() helper so the
forms cannot diverge again.
@EfeDurmaz16
EfeDurmaz16 force-pushed the fix/canonical-json-non-ascii branch from 34067f2 to 742c1f4 Compare July 28, 2026 14:40
Recompute the challenge id the way a peer verifier does, from the `request`
value parsed out of the WWW-Authenticate header, and assert it matches the
id the server minted.

The existing unicode vector only exercises generate_challenge_id(), which
was already correct, so nothing covered the wire form.
@EfeDurmaz16
EfeDurmaz16 force-pushed the fix/canonical-json-non-ascii branch from 742c1f4 to 754de0b Compare July 28, 2026 14:45
@EfeDurmaz16

Copy link
Copy Markdown
Author

@brendanjryan could you take a look, or point me at the right reviewer?

Short version: for any non-ASCII byte in the request, the challenge id is computed over one serialization and the header carries another, so a peer verifier cannot reproduce the id. pympp still verifies its own challenges because verify() re-serializes the decoded request, which is why nothing caught it locally.

Workflows are sitting at action_required, so CI has not run on this yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant